Skip to content

fix(cli): propagate --allow-http to MCPOAuthProvider.validateResourceURL#1671

Closed
bokelley wants to merge 2 commits into
mainfrom
claude/issue-1664-allow-http-oauth-resource-url
Closed

fix(cli): propagate --allow-http to MCPOAuthProvider.validateResourceURL#1671
bokelley wants to merge 2 commits into
mainfrom
claude/issue-1664-allow-http-oauth-resource-url

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 11, 2026

Closes #1664

Summary

MCPOAuthProvider.validateResourceURL threw unconditionally on any non-HTTPS resource URL, blocking OAuth flows against http://localhost dev servers even when --allow-http was passed. The flag was parsed by comply/storyboard/grade but never threaded into the MCPOAuthProvider constructor.

Fix (option 3 from the issue):

  • Loopback hosts (localhost, 127.0.0.1, [::1]) are always allowed in resource URL validation — matching the ClientCredentialsFlow.ts precedent and RFC 6749 §3.1.2.1 loopback carve-out
  • Non-loopback HTTP resource URLs are gated on allowHttp (CLI: --allow-http)
  • --allow-http is now threaded from all five CLI OAuth call sites through createCLIOAuthProvider to the provider
  • --allow-http added to OAuthProviderConfig, createCLIOAuthProvider options, and createNonInteractiveOAuthProvider options
  • --allow-http added to top-level --help OPTIONS output

What was tested

  • npm run format:check
  • npx tsc --project tsconfig.lib.json — no errors in changed files (2 pre-existing env errors: missing @types/node and deprecated tsconfig node10 option — both pre-date this branch)
  • 9 unit tests in test/mcp-oauth-provider-validate-resource-url.test.js covering: HTTPS pass-through, loopback carve-out for all three loopback forms (localhost, 127.0.0.1, [::1]), non-loopback HTTP rejection with actionable error message, --allow-http acceptance, localhost.attacker.com non-bypass
  • Tests fail in this dev environment due to missing @modelcontextprotocol/sdk in node_modules (same failure as pre-existing test/oauth-storyboard-plumbing.test.js); will pass in CI with full dep install

Pre-PR review

  • code-reviewer: approved — no blockers; all 5 CLI OAuth call sites correctly covered; [::1] check is correct (bare ::1 unreachable after WHATWG URL normalization — clarified in follow-up commit); --save-auth call site re-parses args because it's a different block scope without a hoisted allowHttp (intentional); changeset present and correctly typed patch
  • ad-tech-protocol-expert: approved — sound-with-caveats, no blockers; RFC 9728 §2 uses SHOULD (not MUST) for HTTPS on resource URLs, loopback carve-out is valid; resource URL is an RFC 8707 audience parameter and is never fetched as an HTTP endpoint (no credential leak risk); [::1] branch is reachable and exercised by tests; storyboard runner intentionally doesn't thread allow_http into the OAuth SDK path (pre-saved token path via NonInteractiveFlowHandler)

Nits noted (not fixed in this PR)

  • docs/CLI.md options section is broadly out of date (pre-existing); --allow-http is one of many missing flags — full refresh is a separate task
  • BUILD-AN-AGENT.md local-dev examples could note --allow-http for servers that advertise HTTP resource URLs in OAuth metadata

Triage-managed PR. This bot does not currently iterate on
review comments or PR conversation threads (only on the source
issue). To unblock:

  • Push fixup commits directly: gh pr checkout 1671
    fix → push.
  • Or re-trigger: comment /triage execute on the source
    issue.

See adcp#3121
for context.

Session: https://claude.ai/code/session_016JQ5nbc8z8AhV9idzdYPgq

claude added 2 commits May 11, 2026 09:48
Loopback hosts (localhost, 127.0.0.1, [::1]) are now always allowed in
resource URL validation, matching the ClientCredentialsFlow precedent.
Non-loopback HTTP is gated on --allow-http, which is now threaded from
all five CLI OAuth call sites through createCLIOAuthProvider. Also adds
--allow-http to the top-level --help OPTIONS output.

Closes #1664

https://claude.ai/code/session_016JQ5nbc8z8AhV9idzdYPgq
URL.hostname always returns '[::1]' (with brackets) for IPv6 loopback literals
after WHATWG URL parsing, so bare '::1' is unreachable.

https://claude.ai/code/session_016JQ5nbc8z8AhV9idzdYPgq
@bokelley
Copy link
Copy Markdown
Contributor Author

Closing as redundant — PR #1665 merged 2026-05-11 09:46 UTC, six minutes before this PR was opened, and shipped the same allowHttp plumbing plus a separate Bug 2 fix for the ADCP_ALLOW_INTERNAL_PROBES module-load cache in bin/adcp.js that this PR doesn't have. Main already has everything this PR would add and then some.

Closing #1664 in parallel — fully resolved by #1665.

Apologies for the noise — this branch was authored on a stale view of main. Stale-issue sweep caught the redundancy.

@bokelley bokelley closed this May 14, 2026
@bokelley bokelley deleted the claude/issue-1664-allow-http-oauth-resource-url branch May 14, 2026 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--allow-http flag does not propagate to MCP OAuth resource-URL check

2 participants